Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback in user_for_paper_trail to accept current_user returning a string #346

Closed
wants to merge 1 commit into from

Conversation

shweelan
Copy link
Contributor

3.0.1 introduced a breaking changes to how current_user works. This is a workaround for backward compatibility.

@batter
Copy link
Collaborator

batter commented Mar 19, 2014

It's shouldn't be a breaking change because you can always override the user_for_paper_trail method on a per-controller basis (or globally by defining it on your ApplicationController) if you want it to behave differently. Please see #316 for context as to why the change was made.

This seems reasonable I suppose, but what is your current_user object returning? The reason this change was made was because most people have an User model instance or something to that effect being returned from their current_user helper. You just have a String?

@batter batter added this to the 3.0.2 milestone Mar 19, 2014
@batter batter self-assigned this Mar 19, 2014
@shweelan
Copy link
Contributor Author

I have multiple application toolbox, they are commonly using the current_user function which returning only string not a model object, because I am using LDAP active directory credentials for users login, that's why I don't have users model. my changes to paper_trail gem will provide more flexibility to use string as well as model object.

@batter batter closed this in a0b93ab Mar 20, 2014
@batter
Copy link
Collaborator

batter commented Mar 20, 2014

So I had to switch up the invocation of the try method to accommodate the change in behavior between ActiveSupport 3 & 4, but I merged this in. Thanks for the PR.

I still would argue it's not a breaking change, because the user_for_paper_trail method is designed to be overridden by a user, but this will hopefully make that helper method work a little better out of the box for most users.

@shweelan
Copy link
Contributor Author

Hi,
thanks for your kind action.

when I first used paper trail, I felt lucky to get my current_user to the revisions log, with no need for changing any line in my code. That's why I said its breaking change cause when I made bundle update the app crashed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants